Skip to content

chore: promote staging to staging-promote/71f41dd1-23309993684 (2026-03-19 19:18 UTC)#1425

Merged
henrypark133 merged 7 commits intostaging-promote/71f41dd1-23309993684from
staging-promote/52ca9d65-23312673755
Mar 20, 2026
Merged

chore: promote staging to staging-promote/71f41dd1-23309993684 (2026-03-19 19:18 UTC)#1425
henrypark133 merged 7 commits intostaging-promote/71f41dd1-23309993684from
staging-promote/52ca9d65-23312673755

Conversation

@ironclaw-ci
Copy link
Contributor

@ironclaw-ci ironclaw-ci bot commented Mar 19, 2026

Auto-promotion from staging CI

Batch range: 71f41dd12363497372864bc6eb3f7c334e05fd52..52ca9d6588f31fc9b6007c56ed7cd1995d5ad0df
Promotion branch: staging-promote/52ca9d65-23312673755
Base: staging-promote/71f41dd1-23309993684
Triggered by: Staging CI batch at 2026-03-19 19:18 UTC

Commits in this batch (2):

Current commits in this promotion (5)

Current base: staging-promote/71f41dd1-23309993684
Current head: staging-promote/52ca9d65-23312673755
Current range: origin/staging-promote/71f41dd1-23309993684..origin/staging-promote/52ca9d65-23312673755

Auto-updated by staging promotion metadata workflow

Waiting for gates:

  • Tests: pending
  • E2E: pending
  • Claude Code review: pending (will post comments on this PR)

Auto-created by staging-ci workflow

nearfamiliarcow and others added 2 commits March 19, 2026 11:45
…requests (#1257)

The HTTP tool returned `ApprovalRequirement::Always` for requests with
credentials, but `Always` is hardcoded to ignore the session auto-approve
set. This meant users who clicked "always" were re-prompted on every
subsequent HTTP call — the UI offered "always" but the backend ignored it.

Two fixes:
1. HTTP credentialed requests now return `UnlessAutoApproved` instead of
   `Always`, so the session auto-approve set is respected.
2. `StatusUpdate::ApprovalNeeded` now carries `allow_always: bool`. All
   channel UIs (Telegram, Slack, Signal, REPL, Web) conditionally hide
   the "always" option when a tool truly requires per-invocation approval
   (`ApprovalRequirement::Always`, e.g. destructive shell commands).

Also boxes `PendingApproval` in `AgenticLoopResult::NeedApproval` to fix
a pre-existing clippy `large_enum_variant` warning.

Regression tests included (test_credentialed_requests_respect_auto_approve,
test_allow_always_matches_approval_requirement) but CI heuristic cannot
detect them in cross-fork PR diffs.

[skip-regression-check]

Co-authored-by: Tyler <tbond@tbond-m5.local>
* feat: receive relay events via webhook callbacks instead of SSE

Replace the SSE pull model with push-based webhook callbacks from
channel-relay. Eliminates the reconnect loop, stream token auth,
and SSE parser — events arrive via HTTP POST to /relay/events.

- Add webhook handler with HMAC signature verification
- Simplify RelayChannel to use mpsc from webhook handler
- Remove SSE connect/reconnect/parse logic from RelayClient
- Add register_callback() to RelayClient for callback URL registration
- Update activation flow to create event channel and register callback
- Wire relay webhook endpoint into web gateway

* fix: address review feedback on webhook callback PR

- Return 503 when relay event channel is full/closed (enables retry)
- Reject malformed timestamps with 400 instead of proceeding
- Allow relay activation without settings store (no-store/ephemeral mode)
- Check installed_relay_extensions set in is_relay_channel for no-db mode
- Fix staging test constructors for new RelayChannel signature

* security: adapt relay client to new channel-relay auth model

Adapts the relay integration to the hardened channel-relay security model:

- Switch from X-API-Key header to Authorization: Bearer sk-agent-*
  for all relay API calls (chat-api token verification)
- Remove register_callback() — PUT /callbacks endpoint removed
- Remove event_callback_url from initiate_oauth() — parameter removed
- Make signing_secret a required field in RelayConfig (new env var:
  CHANNEL_RELAY_SIGNING_SECRET)
- Update integration tests for Bearer auth and removed endpoints

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: use server-side approval tokens, remove caller-supplied routing

- Approval flow now calls POST /approvals to register server-side
  record, then embeds only the opaque approval_token in button value
- Remove instance_id parameter from proxy_provider() — channel-relay
  no longer accepts it (uses verified identity)
- Remove instance_id and user_id from initiate_oauth() — channel-relay
  derives them from the Bearer token
- Add create_approval() to RelayClient

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: pass webhook_url during OAuth so callback_url is set on connection

The channel-relay OAuth flow now accepts webhook_url to set the
callback_url during connection creation. IronClaw computes its webhook
URL from callback_base + webhook_path and passes it during initiate_oauth.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: remove webhook_url from OAuth initiation

Channel-relay now derives the callback URL from chat-api's instance_url.
IronClaw no longer supplies webhook_url during OAuth — the relay is the
authority on where events get delivered.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* chore: cargo fmt

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: remove all URL params from OAuth initiation

IronClaw no longer supplies any URLs to channel-relay. The relay
derives all URLs from the trusted instance_url in chat-api.
initiate_oauth() takes no parameters.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: restore CSRF nonce for OAuth callback validation

Re-add nonce generation and secret storage in auth_channel_relay.
The nonce is passed to channel-relay as state_nonce param (not a URL).
Channel-relay embeds it in the signed state and appends it to the
redirect URL so IronClaw's callback handler can validate and activate.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: per-instance callback signing secrets

relay_signing_secret() now prefers OPENCLAW_GATEWAY_TOKEN (per-instance)
over the shared CHANNEL_RELAY_SIGNING_SECRET. A compromised instance
can no longer forge callbacks to other instances on the same relay.
CHANNEL_RELAY_SIGNING_SECRET is now optional in RelayConfig.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: clean per-instance callback secrets, no shared secrets, no fallbacks

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: pass team_id to get_signing_secret for workspace-scoped lookup

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* security: remove sender_id from create_approval — relay derives it

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: remove stale relay sender_id validation

* fix: harden relay webhook activation lifecycle

---------

Co-authored-by: Pierre <pierre@near.ai>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@github-actions github-actions bot added scope: agent Agent core (agent loop, router, scheduler) scope: channel Channel infrastructure scope: channel/web Web gateway channel scope: channel/wasm WASM channel runtime scope: tool/builtin Built-in tools scope: extensions Extension management size: XL 500+ changed lines risk: medium Business logic, config, or moderate-risk modules contributor: core 20+ merged PRs labels Mar 19, 2026
@claude
Copy link

claude bot commented Mar 19, 2026

Code review

No issues found.

This PR implements two well-integrated changes:

  1. Relay webhook refactoring (feat: receive relay events via webhook callbacks feat: receive relay events via webhook callbacks #1254): Cleanly transitions from SSE long-polling to webhook callbacks with proper HMAC-SHA256 signature verification, timestamp freshness validation (5-min window), and correct hex decoding + 32-byte length validation of signing secrets. The webhook handler at POST /relay/events is well-designed with proper error responses at each validation step.

  2. Approval system enhancement (fix: make "always" auto-approve work for credentialed HTTP requests fix(approval): make "always" auto-approve work for credentialed HTTP requests #1257): Correctly changes ApprovalRequirement::AlwaysUnlessAutoApproved for credentialed HTTP requests, allowing the session-wide "always" setting to apply. The allow_always field is properly threaded through PendingApproval, SubmissionResult, and all approval handlers (dispatcher, thread_ops, channel), with regression tests verifying the behavior.

All code follows CLAUDE.md guidelines: no unwrap()/expect() in production code, proper error context mapping, and strong type usage. Configuration simplification removes SSE polling parameters cleanly. Test cases are comprehensive and updated appropriately.

@claude
Copy link

claude bot commented Mar 19, 2026

Code review - Architecture findings

Found 7 architectural improvement opportunities (non-blocking):

  1. [MEDIUM:75] Stringly-typed approval flag logic instead of enum method

    • File: src/agent/dispatcher.rs:590, src/agent/thread_ops.rs:1090
    • The allow_always = !matches!(requirement, ApprovalRequirement::Always) pattern is duplicated across 3+ places. Should extract as ApprovalRequirement::allows_always() method to follow DRY principle and CLAUDE.md preference for strong types over computed booleans.
  2. [MEDIUM:65] Optional state accessed via match/if-let chains in relay webhook handler

    • File: src/channels/web/server.rs:765-820
    • Multiple guard clauses return HTTP errors inline. Consider extracting a RelayCallbackError type with thiserror to centralize validation logic and make it testable.
  3. [MEDIUM:60] Inconsistent error handling for mutex lock failures

    • File: src/extensions/manager.rs:597 (relay_signing_secret_cache)
    • Using lock().ok()? conflates lock failure (poison) with cache miss. Lock failure should panic/expect; cache miss is normal. Consider: self.relay_signing_secret_cache.lock().expect("lock poisoned").clone()
  4. [MEDIUM:65] API leaks internal Arc/Mutex types

    • File: src/extensions/manager.rs:578-587 (relay_event_tx getter)
    • Returns Arc<tokio::sync::Mutex<Option<...>>> directly, coupling callers to internal sync strategy. Consider offering a higher-level abstraction (e.g., get_relay_event_tx() that returns sender directly).
  5. [LOW:55] Duplicated allow_always computation

    • File: src/agent/dispatcher.rs:553-591, src/agent/thread_ops.rs:1072-1099
    • Same logic appears in both approval flows. Extract as helper to ensure consistency.
  6. [LOW:50] Stringly-typed state keys without constants

    • File: src/extensions/manager.rs (throughout)
    • Keys like "relay:{}:team_id" and "relay:{}:oauth_state" are hard-coded. Define as module constants to prevent typos and enable refactoring.
  7. [LOW:45] Error context not propagated in relay initialization

    • File: src/extensions/manager.rs:4007-4014
    • client.get_signing_secret() errors are wrapped as ExtensionError::Config without preserving the error chain. Use .map_err() to add context for better debuggability.

Non-issues: No production .unwrap()/.expect() violations. All error types use thiserror. Bearer auth strategy is consistent.

@claude
Copy link

claude bot commented Mar 19, 2026

Code review - Security & Safety findings

Completed security analysis. No critical vulnerabilities found.

[MEDIUM:65] Timestamp replay window for webhook callbacks

  • File: src/channels/web/server.rs:1854-1864
  • The webhook signature verification checks timestamp freshness (5-minute window) but does not implement replay protection. An attacker could capture a valid signed webhook and replay it multiple times within the window. The HMAC signature includes the timestamp, preventing trivial replay, but distributed replay within the window is theoretically possible. Recommend: Consider adding a nonce cache or sequence number tracking if high-value operations depend on webhook uniqueness.

[MEDIUM:50] Race condition window in relay event sender access

  • File: src/channels/web/server.rs:1886-1898
  • Between checking if relay channel is active and sending the event, a concurrent deactivate_relay_channel() could remove the sender. Mitigation: Code already handles this via try_send() error return and responds with 503. This is acceptable.

[LOW:40] Sensitive bytes in Arc could expose in debug panics

  • File: src/extensions/manager.rs:relay_signing_secret_cache
  • The 32-byte signing secret is stored in Arc<Mutex<Option<Vec>>>. While unlikely to be logged, consider using zeroize crate for sensitive byte arrays to prevent accidental memory exposure on panic.

[MEDIUM:50] Validation assumption on cached signing secret

  • File: src/extensions/manager.rs:relay_signing_secret()
  • The cached secret is assumed valid because it was validated by get_signing_secret() at fetch time. Since the cache is only populated by this method, invariant holds, but the assumption is implicit. Adding a comment explaining this invariant would clarify intent.

Verified Strengths:
✅ HMAC-SHA256 signature verification uses constant-time comparison (subtle::ConstantTimeEq)
✅ Bearer token auth properly used (not API keys in headers)
✅ No .unwrap()/.expect() in production webhook code
✅ Proper error handling on all validation steps

Conclusion: Webhook refactoring is well-implemented with no blocking security issues. Replay protection is a design consideration rather than a bug.

@claude
Copy link

claude bot commented Mar 19, 2026

Code review - Performance & Production findings

Found 6 performance/production considerations:

[HIGH:80] Synchronous mutex in async hot path

  • File: src/channels/web/server.rs:771 (relay_events_handler)
  • The webhook handler calls ext_mgr.relay_signing_secret() which uses std::sync::Mutex::lock() on every incoming event. This is a blocking synchronous lock acquired in an async context, which can block the tokio runtime under contention. Recommendation: Use Arc<tokio::sync::Mutex<_>> instead of Arc<std::sync::Mutex<_>> for relay_signing_secret_cache.

[HIGH:75] Lock contention on event sender per-request

  • File: src/channels/web/server.rs:831 (relay_events_handler)
  • Every webhook event acquires async lock on relay_event_tx via ext_mgr.relay_event_tx().lock().await. Under sustained webhook traffic, this per-request lock becomes a serialization bottleneck. Recommendation: Clone sender once at activation and store in lock-free cell (e.g., AtomicOption<Arc<Sender>>) to avoid repeated lock acquisitions.

[MEDIUM:65] Channel capacity mismatch between test and production

  • File: src/extensions/manager.rs:6435 (test setup vs production)
  • Test creates mpsc::channel(1) but production uses mpsc::channel(64). This capacity-1 test channel will cause try_send() to fail immediately under load, triggering 503 responses. Test doesn't validate the queue backpressure behavior that production encounters. Recommendation: Update test to capacity 64 to match production, or document intentional difference.

[MEDIUM:50] Unnecessary cloning of 32-byte signing secret

  • File: src/extensions/manager.rs (activation and verification paths)
  • The signing secret is cloned during cache storage (*cache = Some(signing_secret)) and again on retrieval (.clone()). While 32 bytes is small, under high webhook volume this adds allocations in the hot path. Recommendation: Use Arc<[u8; 32]> instead of Vec<u8> to eliminate allocations.

[LOW:60] Lost resilience in relay service recovery

  • File: src/channels/relay/channel.rs (removed ~200 lines of SSE reconnection logic)
  • Old SSE implementation had exponential backoff (1s→60s) and token renewal. New webhook approach has no recovery if relay service is temporarily unavailable. Webhook events fail with "event queue full" until manual restart. Recommendation: Add periodic health checks (e.g., list_connections()) to detect relay service degradation.

[LOW:40] Approval tuple repeatedly extracted for channel updates

  • File: src/agent/thread_ops.rs:507-527
  • The approval request is extracted multiple times for channel status broadcasts, causing repeated field access. Recommendation: Extract tool_name/description once and reuse.

Positive notes: No unbounded loops, missing timeouts, or resource leaks detected. Event channel sized appropriately at 64-event buffer. Bearer auth prevents token injection attacks in headers.

Summary: 2 high-priority async/concurrency improvements recommended to prevent bottlenecks under sustained webhook traffic. Remaining findings are minor optimizations.

ilblackdragon and others added 5 commits March 19, 2026 13:37
* feat: LRU embedding cache for workspace search (#165)

Add CachedEmbeddingProvider that wraps any EmbeddingProvider with an
in-memory LRU cache keyed by SHA-256(model_name + text). This avoids
redundant HTTP calls when the same text is embedded multiple times
(common during reindexing and repeated searches).

- Cache uses HashMap + last_accessed tracking with manual LRU eviction
  (same pattern as llm::response_cache::CachedProvider)
- Lock is never held during HTTP calls to prevent blocking
- embed_batch() partitions into hits/misses and only fetches misses
- Default 10,000 entries (~58 MB for 1536-dim vectors)
- Configurable via EMBEDDING_CACHE_SIZE env var
- Workspace.with_embeddings() auto-wraps; with_embeddings_uncached()
  available for tests

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address review comments on embedding cache

- Validate embed_batch return count matches expected miss count
- Replace unwrap_or_default() with proper error propagation
- Fix batch eviction: run final eviction pass after insert to enforce cap
- Fix test: use different-length inputs to verify ordering correctness
- Reject EMBEDDING_CACHE_SIZE=0 in config validation (minimum is 1)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: replace .expect() with proper error handling in embed_batch

The all-cache-hits early-return path used .expect("all cache hits") which
violates the project convention of no .unwrap()/.expect() in production
code. Replaced with the same ok_or_else pattern used in the normal path.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: clarify memory sizing docs and use saturating_add for eviction

- Update memory comments in embedding_cache.rs, config/embeddings.rs,
  and workspace/mod.rs to note the ~58 MB figure is payload-only
  (actual memory is higher due to HashMap/key/allocation overhead)
- Use saturating_add(1) instead of + 1 for eviction threshold to
  prevent overflow if max_entries is usize::MAX

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address Copilot review on embedding cache

- Avoid double-clone per miss in embed_batch: move embedding into
  results, clone only for the cache entry
- Evict per-insert instead of after all inserts to keep peak memory
  bounded during large batches
- Clamp max_entries to at least 1 in constructor to prevent unexpected
  eviction behavior when set to 0 via the public API

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: reduce embedding_cache module visibility to private

Types are already re-exported via `pub use`, so the module itself
doesn't need to be public. Reduces unnecessary API surface.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address serrrfirat review feedback on embedding cache

- Add TODO comment for O(n) LRU eviction scalability
- Add thundering herd note at lock release in embed()
- Warn when cache max_entries exceeds 100k
- Use with_embeddings_uncached() in integration test
- Add tests: error_does_not_pollute_cache, embed_batch_empty_input
- Update README with cache-aware with_embeddings() docs

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: prevent u32 wrapping in FailThenSucceedMock failure counter

fetch_sub(1) wraps to u32::MAX when called past zero, silently
breaking the mock for 3+ calls. Switch to load-then-store to avoid
the wrapping bug in both embed() and embed_batch().

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address Copilot and serrrfirat review findings on embedding cache

- Switch tokio::sync::Mutex to std::sync::Mutex (lock never held across
  .await — cheaper synchronous lock)
- Extract DEFAULT_EMBEDDING_CACHE_SIZE constant to avoid 10_000 duplication
  between EmbeddingCacheConfig and EmbeddingsConfig

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: add all-misses batch test for embedding cache

Adds embed_batch_all_misses test covering the case where every text in a
batch is a cache miss — fulfilling the commitment from serrrfirat's review.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: trigger CI re-check after rebase

* fix: use raw [u8;32] cache keys and pre-allocate HashMap capacity

Address Copilot review findings:
- cache_key() now returns [u8; 32] instead of hex String, avoiding a
  64-byte allocation per lookup
- HashMap::with_capacity(max_entries) avoids incremental reallocation
- Fix pre-existing staging compilation error in cli/routines.rs
  (missing max_tool_rounds/use_tools fields)

[skip-regression-check]

* fix: make cache accessors sync and update doc for [u8;32] keys

Address Copilot review:
- len(), is_empty(), clear() are now sync since they only take a
  std::sync::Mutex lock with no .await points
- Update cache_size doc comment to reflect [u8;32] keys instead of
  String keys

[skip-regression-check]

* fix: remove clone_on_copy for [u8; 32] cache keys

[skip-regression-check]

* ci: add safety comments to test code for no-panics check

The CI no-panics grep check cannot distinguish test code inside
src/ files from production code. Add // safety: test annotations
to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules.

* fix: correct cache doc and demote hit/miss logs to trace

- Fix misleading "String keys" in memory comment (cache uses [u8; 32])
- Demote per-request hit/miss logs from debug to trace to reduce noise
  on hot paths (batch summary stays at trace too)

* docs: add missing Arc import in workspace README example

* perf: batch eviction in embed_batch to avoid O(n×m) cost

Replace per-insert evict_lru call with a single evict_k_oldest pass
that computes eviction count upfront and removes the k oldest entries
in one O(n) scan. Avoids O(n×m) HashMap iterations while holding the
mutex during batch inserts.

* fix: cap batch cache inserts at max_entries and use O(n) selection

- evict_k_oldest now uses select_nth_unstable_by_key for O(n) average
  partial selection instead of O(n log n) full sort
- embed_batch caps cached entries at max_entries when misses exceed
  capacity, preventing the cache from growing unbounded
- Added test: batch_exceeding_capacity_respects_max_entries

* fix: flatten test assert for fmt compatibility

Shorten assert message to fit single line so cargo fmt doesn't
split the safety annotation onto a separate line.

* fix: address review feedback and improve embedding cache (takeover #235)

- Fix merge conflict: add missing allow_always field in PendingApproval
- Thread EmbeddingCacheConfig through CLI memory commands so they respect
  EMBEDDING_CACHE_SIZE instead of silently using default (fixes #235 review)
- Cap HashMap pre-allocation at min(max_entries, 1024) to avoid upfront
  memory waste at large cache sizes
- Fix FailThenSucceedMock race: replace load+store with atomic fetch_update
- Remove noisy '// safety: test' comments (40+ lines of diff noise)
- Fix collapsed lines from comment removal
- Simplify redundant Ok(...collect()?) to just collect()

Co-Authored-By: ztsalexey <ztsalexey@users.noreply.github.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix(embedding-cache): skip eviction on concurrent duplicate insert

When the lock is released for the HTTP call, another caller may insert
the same key. Re-check under lock and just update the existing entry
without evicting, avoiding unnecessary cache churn under concurrency.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: ztsalexey <alexthebuildr@gmail.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: ztsalexey <ztsalexey@users.noreply.github.com>
* feat: structured fallback deliverables for failed/stuck jobs (#221)

When a job fails or gets stuck, build a FallbackDeliverable that captures
partial results, action statistics, cost, timing, and repair attempts.
This replaces opaque error strings with structured data users can act on.

- Add FallbackDeliverable, LastAction, ActionStats types in context/fallback.rs
- Store fallback in JobContext.metadata["fallback_deliverable"] on failure
- Surface fallback in job_status tool output and SSE job_result events
- Update mark_failed() and mark_stuck() in worker to build fallback
- 8 unit tests covering zero/mixed actions, truncation, timing, serialization

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address review comments on fallback deliverables

- Fix doc comment: "200 chars" -> "200 bytes (UTF-8 safe)" since
  truncate_str operates on byte length, not character count.
- Add code comment documenting that SSE fallback_deliverable is
  currently always None (forward-compatible infrastructure).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* refactor: take Option<&FallbackDeliverable> instead of &Option<…>

Addresses Gemini review feedback: idiomatic Rust prefers
Option<&T> over &Option<T> for borrowed optional values.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: guard against non-object metadata and add fallback test

- store_fallback_in_metadata now resets metadata to {} when it's any
  non-object type (string, array, number), not just null. Prevents
  panic on index assignment.
- Add test_job_status_includes_fallback_deliverable to verify the
  fallback field is surfaced in job_status tool output.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: use sanitized output in fallback preview + add integration tests

Security fix: FallbackDeliverable::build() now uses output_sanitized
instead of output_raw, preventing secrets/PII from leaking through
the job_status tool and SSE job_result events.

Also adds:
- test_fallback_uses_sanitized_output: proves raw secrets don't leak
- test_store_fallback_in_metadata_roundtrip: full serialize/deserialize
- test_store_fallback_handles_non_object_metadata: edge case coverage
- test_store_fallback_none_is_noop: None input is safe

Addresses serrrfirat review feedback on PR #236.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: harden fallback deliverables against review findings

- Truncate failure_reason to 1000 bytes to prevent metadata bloat
- Add tracing::warn on fallback serialization failure (was silently discarded)
- Fix module/struct docs to cover stuck jobs, remove stale SSE claim
- Fix job.rs test to use real FallbackDeliverable field names
- Add tests for failure_reason truncation and completed_at=None elapsed time
- Fix pre-existing clippy warning in settings.rs (field_reassign_with_default)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: address Copilot review findings on fallback deliverables

- Fix output_raw/output_sanitized field swap in ActionRecord::succeed()
  so sanitized data actually goes into the sanitized field (security)
- Return None instead of empty Memory when get_memory fails in
  build_fallback, with tracing::warn for observability
- Replace manual elapsed calculation with ctx.elapsed() which already
  clamps negative durations

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix: resolve rebase conflicts and update tests for parameter swap

- Add fallback field to SseEvent::JobResult in job_monitor
- Fix type annotation in fallback deliverable test
- Update test_action_record_succeed_sets_fields for new parameter order
- Use create_job_for_user in test (API changed on main)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* chore: trigger CI re-check after rebase

* fix: fall back to error message for failed action output_preview

When the last action is a failed tool call, output_sanitized is None,
leaving output_preview empty. Now falls back to the action's error
message so users see what went wrong.

[skip-regression-check]

* ci: add safety comments to test code for no-panics check

The CI no-panics grep check cannot distinguish test code inside
src/ files from production code. Add // safety: test annotations
to .unwrap(), .expect(), and assert!() calls in #[cfg(test)] modules.

* fix: clarify succeed() doc and avoid clone in output_preview

- Fix doc comment: output_raw is stored as pretty-printed JSON string,
  not a raw JSON value
- Borrow string slice directly in fallback preview to avoid cloning
  potentially large sanitized outputs before truncation

* refactor: reuse floor_char_boundary in truncate_str

Replace hand-rolled UTF-8 boundary logic with existing
crate::util::floor_char_boundary to reduce duplication.

* fix: rename SSE fallback field to fallback_deliverable for consistency

The SSE JobResult field was named `fallback` while everywhere else
(metadata key, job_status tool) uses `fallback_deliverable`. Align
the SSE wire format to avoid forcing clients to handle two names.

---------

Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* Make hosted OAuth and MCP auth generic

* Address PR feedback and lint issues

* Suppress built-in Google secret in hosted proxy flows

* Align hosted OAuth secret suppression with proxy config

* Harden hosted OAuth callback helpers

* Tighten hosted OAuth URL rewriting
…4063

chore: promote staging to staging-promote/65062f3c-23317058602 (2026-03-19 23:06 UTC)
…8602

chore: promote staging to staging-promote/52ca9d65-23312673755 (2026-03-19 21:10 UTC)
@github-actions github-actions bot added scope: channel/cli TUI / CLI channel scope: llm LLM integration scope: workspace Persistent memory / workspace scope: orchestrator Container orchestrator scope: worker Container worker scope: docs Documentation labels Mar 20, 2026
@henrypark133 henrypark133 merged commit e031d82 into staging-promote/71f41dd1-23309993684 Mar 20, 2026
13 checks passed
@henrypark133 henrypark133 deleted the staging-promote/52ca9d65-23312673755 branch March 20, 2026 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor: core 20+ merged PRs risk: medium Business logic, config, or moderate-risk modules scope: agent Agent core (agent loop, router, scheduler) scope: channel/cli TUI / CLI channel scope: channel/wasm WASM channel runtime scope: channel/web Web gateway channel scope: channel Channel infrastructure scope: docs Documentation scope: extensions Extension management scope: llm LLM integration scope: orchestrator Container orchestrator scope: tool/builtin Built-in tools scope: worker Container worker scope: workspace Persistent memory / workspace size: XL 500+ changed lines staging-promotion

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants